libxl: add statup checks to libxl__wait_for_device_model

When the device model is starting up push checks for spawn failure down into
libxl__wait_for_device_model, allowing us to fail more quickly when the device
model fails to start (e.g. due to a missing library or an early setup error
etc).

In order to allow the select loop in libxl__wait_for_device_model to wake when
the child dies add pipe between the parent and the intermediate process which
the intermediate process can use to signal the parent.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index ccf6518a896d8bf1f4bb8b5253f76e0e4410adcc..a096d60f3d72bc29ef6c638878e9f93fb6d90f06 100644 (file)
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -522,7 +522,8 @@ int libxl_domain_unpause(libxl_ctx *ctx, uint32_t domid)
         state = libxl__xs_read(&gc, XBT_NULL, path);
         if (state != NULL && !strcmp(state, "paused")) {
             libxl__xs_write(&gc, XBT_NULL, libxl__sprintf(&gc, "/local/domain/0/device-model/%d/command", domid), "continue");
-            libxl__wait_for_device_model(&gc, domid, "running", NULL, NULL);
+            libxl__wait_for_device_model(&gc, domid, "running",
+                                         NULL, NULL, NULL);
         }
     }
     ret = xc_domain_unpause(ctx->xch, domid);

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 5d85822620de5f0cc4ac1d2fa5c6f2916626656d..a3826bdc1cac0d6591cc9bc6e824bae2246f356e 100644 (file)
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -408,6 +408,7 @@ out:
 
 int libxl__wait_for_device_model(libxl__gc *gc,
                                  uint32_t domid, char *state,
+                                 libxl__device_model_starting *starting,
                                  int (*check_callback)(libxl__gc *gc,
                                                        uint32_t domid,
                                                        const char *state,
@@ -437,7 +438,17 @@ int libxl__wait_for_device_model(libxl__gc *gc,
     tv.tv_sec = LIBXL_DEVICE_MODEL_START_TIMEOUT;
     tv.tv_usec = 0;
     nfds = xs_fileno(xsh) + 1;
+    if (starting && starting->for_spawn->fd > xs_fileno(xsh))
+        nfds = starting->for_spawn->fd + 1;
+
     while (rc > 0 || (!rc && tv.tv_sec > 0)) {
+        if ( starting ) {
+            rc = libxl__spawn_check(gc, starting->for_spawn);
+            if ( rc ) {
+                rc = -1;
+                goto err_died;
+            }
+        }
         p = xs_read(xsh, XBT_NULL, path, &len);
         if ( NULL == p )
             goto again;
@@ -459,15 +470,24 @@ again:
         free(p);
         FD_ZERO(&rfds);
         FD_SET(xs_fileno(xsh), &rfds);
+        if (starting)
+            FD_SET(starting->for_spawn->fd, &rfds);
         rc = select(nfds, &rfds, NULL, NULL, &tv);
         if (rc > 0) {
-            l = xs_read_watch(xsh, &num);
-            if (l != NULL)
-                free(l);
-            else
-                goto again;
+            if (FD_ISSET(xs_fileno(xsh), &rfds)) {
+                l = xs_read_watch(xsh, &num);
+                if (l != NULL)
+                    free(l);
+                else
+                    goto again;
+            }
+            if (starting && FD_ISSET(starting->for_spawn->fd, &rfds)) {
+                unsigned char dummy;
+                read(starting->for_spawn->fd, &dummy, sizeof(dummy));
+            }
         }
     }
+err_died:
     xs_unwatch(xsh, path, path);
     xs_daemon_close(xsh);
     LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Device Model not ready");

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index a71d7cfcbe25a31ab5a83b1e84cd58fed5d9e778..7a81cbfe2696233fbd3307f4561cc037d53d5e32 100644 (file)
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -855,14 +855,12 @@ static int detach_device_model(libxl__gc *gc,
     return rc;
 }
 
-
 int libxl__confirm_device_model_startup(libxl__gc *gc,
                                        libxl__device_model_starting *starting)
 {
-    int problem = libxl__wait_for_device_model(gc, starting->domid, "running", NULL, NULL);
     int detach;
-    if ( !problem )
-        problem = libxl__spawn_check(gc, starting->for_spawn);
+    int problem = libxl__wait_for_device_model(gc, starting->domid, "running",
+                                               starting, NULL, NULL);
     detach = detach_device_model(gc, starting);
     return problem ? problem : detach;
 }

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 667b20b0eb56fe2a7ba1c11c605c221956fd506c..f6bca7efb7100f140c5af0dea1641894b4ec4e8f 100644 (file)
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -543,7 +543,7 @@ int libxl__domain_save_device_model(libxl__gc *gc, uint32_t domid, int fd)
 
     LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Saving device model state to %s", filename);
     libxl__xs_write(gc, XBT_NULL, libxl__sprintf(gc, "/local/domain/0/device-model/%d/command", domid), "save");
-    libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL);
+    libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL, NULL);
 
     if (stat(filename, &st) < 0)
     {

diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index 70b54259a6e55147f78134712943b84e6786703d..ce6e93f4f03a47f44634dc12a3d2f03f579cf56b 100644 (file)
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -26,6 +26,7 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <signal.h> /* for SIGKILL */
+#include <fcntl.h>
 
 #include "libxl.h"
 #include "libxl_internal.h"
@@ -91,6 +92,22 @@ void libxl_report_child_exitstatus(libxl_ctx *ctx,
     }
 }
 
+static int libxl__set_fd_flag(libxl__gc *gc, int fd, int flag)
+{
+    int flags;
+
+    flags = fcntl(fd, F_GETFL);
+    if (flags == -1)
+        return ERROR_FAIL;
+
+    flags |= flag;
+
+    if (fcntl(fd, F_SETFL, flags) == -1)
+        return ERROR_FAIL;
+
+    return 0;
+}
+
 int libxl__spawn_spawn(libxl__gc *gc,
                       libxl__spawn_starting *for_spawn,
                       const char *what,
@@ -100,22 +117,33 @@ int libxl__spawn_spawn(libxl__gc *gc,
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     pid_t child, got;
-    int status;
+    int status, rc;
     pid_t intermediate;
+    int pipes[2];
+    unsigned char dummy = 0;
 
     if (for_spawn) {
         for_spawn->what = strdup(what);
         if (!for_spawn->what) return ERROR_NOMEM;
+
+        if (libxl_pipe(ctx, pipes) < 0)
+            goto err_parent;
+        if (libxl__set_fd_flag(gc, pipes[0], O_NONBLOCK) < 0 ||
+            libxl__set_fd_flag(gc, pipes[1], O_NONBLOCK) < 0)
+            goto err_parent_pipes;
     }
 
     intermediate = libxl_fork(ctx);
-    if (intermediate ==-1) {
-        if (for_spawn) free(for_spawn->what);
-        return ERROR_FAIL;
-    }
+    if (intermediate ==-1)
+        goto err_parent_pipes;
+
     if (intermediate) {
         /* parent */
-        if (for_spawn) for_spawn->intermediate = intermediate;
+        if (for_spawn) {
+            for_spawn->intermediate = intermediate;
+            for_spawn->fd = pipes[0];
+            close(pipes[1]);
+        }
         return 1;
     }
 
@@ -124,8 +152,10 @@ int libxl__spawn_spawn(libxl__gc *gc,
     child = fork();
     if (child == -1)
         exit(255);
-    if (!child)
+    if (!child) {
+        if (for_spawn) close(pipes[1]);
         return 0; /* caller runs child code */
+    }
 
     intermediate_hook(hook_data, child);
 
@@ -134,9 +164,23 @@ int libxl__spawn_spawn(libxl__gc *gc,
     got = call_waitpid(ctx->waitpid_instead, child, &status, 0);
     assert(got == child);
 
-    _exit(WIFEXITED(status) ? WEXITSTATUS(status) :
+    rc = (WIFEXITED(status) ? WEXITSTATUS(status) :
           WIFSIGNALED(status) && WTERMSIG(status) < 127
           ? WTERMSIG(status)+128 : -1);
+    if (for_spawn)
+        write(pipes[1], &dummy, sizeof(dummy));
+    _exit(rc);
+
+ err_parent_pipes:
+    if (for_spawn) {
+        close(pipes[0]);
+        close(pipes[1]);
+    }
+
+ err_parent:
+    if (for_spawn) free(for_spawn->what);
+
+    return ERROR_FAIL;
 }
 
 static void report_spawn_intermediate_status(libxl__gc *gc,

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9c77f44740f82e378db72bf19eff5628b1841cce..8d25ef48a21359cf367e00162ea38fa36452cf44 100644 (file)
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -233,6 +233,7 @@ typedef struct {
     /* put this in your own status structure as returned to application */
     /* all fields are private to libxl_spawn_... */
     pid_t intermediate;
+    int fd;
     char *what; /* malloc'd in spawn_spawn */
 } libxl__spawn_starting;
 
@@ -270,6 +271,7 @@ _hidden int libxl__confirm_device_model_startup(libxl__gc *gc,
 _hidden int libxl__detach_device_model(libxl__gc *gc, libxl__device_model_starting *starting);
 _hidden int libxl__wait_for_device_model(libxl__gc *gc,
                                 uint32_t domid, char *state,
+                                libxl__device_model_starting *starting,
                                 int (*check_callback)(libxl__gc *gc,
                                                       uint32_t domid,
                                                       const char *state,

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 6001c6f06ffcf5983d37e749c565dc5105db8fea..c50b74109f9442e26fbc17e86c9064a60f01b2cd 100644 (file)
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -612,7 +612,8 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
 
     hvm = libxl__domain_is_hvm(gc, domid);
     if (hvm) {
-        if (libxl__wait_for_device_model(gc, domid, "running", NULL, NULL) < 0) {
+        if (libxl__wait_for_device_model(gc, domid, "running",
+                                         NULL, NULL, NULL) < 0) {
             return ERROR_FAIL;
         }
         path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid);
@@ -626,7 +627,8 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
                            pcidev->bus, pcidev->dev, pcidev->func);
         path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/command", domid);
         xs_write(ctx->xsh, XBT_NULL, path, "pci-ins", strlen("pci-ins"));
-        rc = libxl__wait_for_device_model(gc, domid, NULL, pci_ins_check, state);
+        rc = libxl__wait_for_device_model(gc, domid, NULL, NULL,
+                                          pci_ins_check, state);
         path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/parameter", domid);
         vdevfn = libxl__xs_read(gc, XBT_NULL, path);
         path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid);
@@ -843,7 +845,8 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
 
     hvm = libxl__domain_is_hvm(gc, domid);
     if (hvm) {
-        if (libxl__wait_for_device_model(gc, domid, "running", NULL, NULL) < 0) {
+        if (libxl__wait_for_device_model(gc, domid, "running",
+                                         NULL, NULL, NULL) < 0) {
             return ERROR_FAIL;
         }
         path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid);
@@ -857,7 +860,8 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
          * device-model for function 0 */
         if ( !force && (pcidev->vdevfn & 0x7) == 0 ) {
             xs_write(ctx->xsh, XBT_NULL, path, "pci-rem", strlen("pci-rem"));
-            if (libxl__wait_for_device_model(gc, domid, "pci-removed", NULL, NULL) < 0) {
+            if (libxl__wait_for_device_model(gc, domid, "pci-removed",
+                                             NULL, NULL, NULL) < 0) {
                 LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Device Model didn't respond in time");
                 /* This depends on guest operating system acknowledging the
                  * SCI, if it doesn't respond in time then we may wish to